Hi All,

I have built a basic protobuf implementation on the mux transport and it turned out to be fairly easy using thriftmux for some inspiration.

I'm now working on getting the tracing to work with the new implementation. I have added Tracers to the DefaultClient and DefaultServer created with the mux transporter but the service name and method name aren't filled in. So it looks like I need to call something like:

  Trace.recordRpcname(serviceName, methodName)

after the TracingFilter executes and pushes the Tracer and sets up the trace id. But I'm not sure how best to insert this call into client and server instances.

I've read through much of the thriftmux implementation and I have been unable to find how it is done there. Is thriftmux recording the service name and method name somehow that I am missing?

It does look like in the DefaultServer the 'prepare' field (ServiceFactory) could be used to record the rpc information. Is that correct? Or is there a better place to do that?

On the client side, I haven't found some sort of hook to use yet. Is there something in DefaultClient I can use? or maybe the ClientDispatcher?

Thanks for the help,
Patrick.

I'm willing to bet that recordRpcname is not called in mux and that's a bug. Lemme look into it some more.

Thanks for the quick response.

My thinking is that the mux transport has no way of knowing what the service name and method name should be. So that is why I was looking for some sort of hook in the client and server to provide the information.

Also on a side note, I've been looking at thriftmux and the fact that the client and server builders aren't compatible with mux and I'm starting to wonder if anyone has used the mux transport in production.

Is mux just experimental at this point or is it production ready?

Thanks again,
Patrick

Also on a side note, I've been looking at thriftmux and the fact that the client and server builders aren't compatible with mux and I'm starting to wonder if anyone has used the mux transport in production.

The intention is to use the new-style APIs; ClientBuilders and ServerBuilders are not going to be provided for new protocols. @roanta has a change coming—it should hit github very soon—which allows the same kind of customization with the new APIs.

Thanks Marius. I understand not using the builders with the new protocols. In the meantime I've passed the tracer into the default client and server.

The driver behind us moving the protobuf implementation to mux is so that we can hook up Zipkin for distributed tracing in production. So I have two primary concerns:

• getting the tracing working with the service and method names
• and knowing that the mux transport is production ready

Do you have some thoughts on that?

Thanks,
Patrick

Both of these issues are huge focusses for the finagle team at twitter, and we're actively responding to any issues that arise with them. I'm not promising it's all bug-free, but we're extra-responsive on this stuff this quarter. You're on the cutting edge, but I don't think you're too experimental to be in production.

Great. Thanks for the info. If I can help with the mux code let me know.

I'm working on integrating the protobuf mux implementation with our existing services to see how the implementation shakes out. I'll let you know if I see any other issues.

Hi @bmdhacks,

Do you have an update on the rpc service name and method name issue with the mux transport?

Should I open an issue?

Thanks,
Patrick

Is there an update on the mux tracing issue? Should I open an issue?

Thanks,
Patrick

It's fixed internally, we're just working on getting a release out with the code.

what is the timeframe for the tracing fix?

-- Patrick

We'll have something up within a week

@PatrickOsborne: Would you mind updating this pull request, or opening a new one with your implementation? We might as well begin the review process while we wait for that fix. Also, I don't think the fix actually blocks landing the patch?

Ok, the mux tracing fix is now pushed

And now it has been merged back into github.

@george-vacariuc we've been thinking about how it's a huge pain that finagle keeps changing underneath your feet so it's hard to merge in finagle-protobuf. We've also been putting similar projects under the finagle [organization][0]. What do you think about the idea of making finagle-protobuf its own project under the finagle org, so that it doesn't have to go through the same stringent code review process? We can also make you an owner of the project, so you'll be able to merge pull requests etc yourself, which makes the most sense since you're the expert on finagle-protobuf.

We'll still be happy to consult on finagle-protobuf, but we won't have to end up in these sinkholes where nothing gets done.

Thoughts?

Moses, that's a good idea. I will check with our compliance people if
they're ok with me working on this and I will follow up with you in the
next week or so.

On Wed, Aug 13, 2014 at 12:00 PM, Moses Nakamura notifications@github.com
wrote:

@george-vacariuc https://github.com/george-vacariuc we've been thinking
about how it's a huge pain that finagle keeps changing underneath your feet
so it's hard to merge in finagle-protobuf. We've also been putting similar
projects under the finagle [organization][0]. What do you think about the
idea of making finagle-protobuf its own project under the finagle org, so
that it doesn't have to go through the same stringent code review process?
We can also make you an owner of the project, so you'll be able to merge
pull requests etc yourself, which makes the most sense since you're the
expert on finagle-protobuf.

We'll still be happy to consult on finagle-protobuf, but we won't have to
end up in these sinkholes where nothing gets done.

Thoughts?

—
Reply to this email directly or view it on GitHub
#91 (comment).

@george-vacariuc any update from your compliance folks?

Hey Moses,

Please accept my apologies for having not followed up; unfortunately I will
not be able to contribute to the project.

--George

On Mon, Dec 29, 2014 at 10:48 AM, Moses Nakamura notifications@github.com
wrote:

@george-vacariuc https://github.com/george-vacariuc any update from
your compliance folks?

—
Reply to this email directly or view it on GitHub
#91 (comment).

For the record, after a conversation with @chrisphelps yesterday I rebased this PR against develop and split finagle-protobuf out into its own project in the Finagle org. It's likely that we'll remove the subproject from this repository, and once that happens I'll close this PR.

Thanks, @travisbrown

finagle-protobuf is now a stand-alone project in the Finagle organization and has been removed as a subproject here.